feat(wizard): shared wizard shell, used by project create and add runtime - #2284
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice refactor. The shared shell is small and legible, the shape it enforces on callers (one field per step, fields own their useInput) is well-documented, and the extraction of toAddRuntimeInput keeps the flag path and wizard path validating through the same code — which is the interesting correctness win here. Position-by-key handling of conditional branches is a nice touch, and it's covered by wizard.test.tsx.
A few things I checked and am happy with:
- Tests use real dependencies.
runtime.screen.test.tsxwrites into a real project viacreateGatewayProjectTestHarnessand asserts the on-disk spec + scaffolded files;wizard.test.tsxdrives a syntheticTestWizardend-to-end rather than mocking the shell. No I/O mocks beyond what the harness already provides. - Double-submit guard is sound.
submittingref is reset infinally, so a retry after an error path still works. ExitOnErrorroutes failures throughexit(error)so they surface via the normal CLI error path instead of a React stack trace — matches how the existing wizards behaved.project createbehaviour is preserved. The old screen test still passes as-is per the PR description; theModelFieldcompound step is kept intentionally, which is the right call given the shell has no focus model.- Telemetry. No new instrumentation, but the existing project handlers aren't instrumented either, so this isn't a regression.
Non-blocking observations (won't ask for changes):
useMemoaroundstepElements/stepswon't actually memoize becausechildrenis a fresh array literal each render. Harmless; the work is trivial.- Success footer label went from
[enter] exit(old create) to[enter] continue(new shared default). Minor copy change; fine either way.
Nothing that needs to change before merging.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2284 +/- ##
============================================
+ Coverage 97.06% 97.08% +0.01%
============================================
Files 569 576 +7
Lines 39322 39519 +197
============================================
+ Hits 38167 38366 +199
+ Misses 1155 1153 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
| // rather than inside renderTui so that a piped or CI run stays headless and | ||
| // reports a missing required flag as the usage error it is, instead of | ||
| // renderTui's "interactive mode requires a TTY". | ||
| export function withTuiWhenInteractive(core: Core, io: AppIO): Middleware { |
There was a problem hiding this comment.
withTuiOnEmptyFlagsAndArgs above opens the TUI even without a TTY, so a piped/CI add runtime would fail with "requires a TTY" instead of the missing --name error. This is that middleware behind a TTY check. create and status each had it hand-rolled; now they share it.
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
jariy17
left a comment
There was a problem hiding this comment.
Really good left some follow up nits
| project.handler(createAddProjectResourceHandler(config)); | ||
| // create wizard; any user-supplied flag, --json, or a non-TTY invocation keeps | ||
| // the headless handler (see withTuiWhenInteractive). | ||
| const tuiWhenInteractive = withTuiWhenInteractive(core, io); |
There was a problem hiding this comment.
I thought the middleware handles when jo flags are passed. Do we still need this?
There was a problem hiding this comment.
You’re right that the existing middleware handles the no-flags check. This wrapper only adds the TTY gate: interactive invocations open the TUI, while non-TTY invocations use the normal headless handler.
If this is being centralized elsewhere, we can remove it here. I can make this change in a follow up PR by adding it into the existing middleware. What do you think?
…time Adds src/components/wizard: a Wizard shell that owns the step list, position, key handling and the form → running → success | error phases, with Step and the TextField/ChoiceField/Summary fields. Fields publish their own key hints and own their step's input handling, so a screen only declares questions. Migrates project create onto it (−380 lines of its own controller) and adds the first add wizard, project add runtime: name → template → description → review. Both paths build their input through one builder — create through resolveScaffoldHarnessInput/resolveRuntimeTemplateShortcut as before, add runtime through the new toAddRuntimeInput — so a value the flag path accepts is not one a wizard rejects. The runtime wizard scaffolds from a template; the JSON infrastructure flags, the model provider/api-key and the Bedrock Agent import stay on the flag path. Also: KeyValueTable grows into the space it is given, so its percentage key cap resolves when it is laid out as a row item, and FormTextInput/ FormRadioGroup render bare when given no name or help text, since the step already asks the question.
…lidation - a bare `project add runtime` on a TTY now opens the wizard: the add router uses withTuiWhenInteractive, which is withTuiOnEmptyFlagsAndArgs behind a TTY gate. Flags, --json and non-TTY runs stay headless. The same middleware replaces the hand-rolled dispatch shims on create and status. - text fields validate the value as typed rather than a trimmed copy, so a step cannot accept " my_agent " and then submit it. - one shared RuntimeNameSchema (42 characters, the memory-name cap less "Memory") behind both --name and the wizard.
The wizard asks only what scaffolding a runtime needs — name and template. --description still sets one on the flag path.
Wizard takes a doneLabel so the success footer can name what enter does; add runtime navigates back instead of exiting, the way build and deploy do.
An add screen the user navigated to should not tear the TUI down over a rejected name: onError=retry reports the failure in the wizard and returns to the form with the answers intact. project create keeps exit.
ea56301 to
9efa8ce
Compare
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
First of a few PRs. This one lands the shared wizard shell and puts two commands (agentcore project create and agetncore project add runtime) on it; the remaining
project addwizards follow separately.What
src/components/wizard/:Wizard: derives the step list from its<Step>children, owns position, key handling and the form → running → success | error phases, and renders the standard Layout + Stepper frame. Position is keyed by step name, so a{condition && <Step/>}branch appearing or disappearing does not move the user. A submit that streamsProgressEvents is rendered with the samedriveProgress+TaskListas create, build and deploy.Step: one question, one field.fields:TextField,ChoiceField,Summary. Each field owns its step'suseInputand publishes its own key hints, so a screen writes questions and nothing else.Callers
project createmoved onto it, unchanged in behavior. Its own controller (phases, step components, success panel, hint plumbing) is deleted; the two-level model step stays as a compound field, since the shell has no notion of focus within a step.project add runtimeis new: name → template → description → review.toAddRuntimeInputis extracted from the handler so the flag path and the wizard validate the same way.The runtime wizard scaffolds from a template. The JSON infrastructure flags,
--model-provider/--api-key(a secret read from stdin orfile://) and the Bedrock Agent import stay on the flag path.Shared components
The fields render existing components rather than new ones:
FormTextInput,FormRadioGroup,KeyValueTable,ErrorPanel,Layout,ui/{stepper,divider,spinner,task-list}. Two fixes at the source:KeyValueTablegrows into the space it is given, so its percentage key cap has a parent width to resolve against when the table is laid out as a row item.FormTextInput/FormRadioGrouprender bare when given no name or help text — the step already asks the question.Testing
bun test3218 pass / 0 fail;tsc --noEmit, oxlint and prettier clean. New:wizard.test.tsxfor the shell,runtime.screen.test.tsxfor the wizard (writes a real project and asserts the spec and scaffolded files),add.screen.test.tsxfor the add menu. Three help-viewport tests inproject.screen.test.tsxmoved toadd payment-manager, sinceadd runtimenow opens a wizard instead of its help.Next
HarnessWizard/EndpointWizardonto the shell (servesharness create/update,endpoint create/update, and givesproject add harnessa screen).